Skip to content

Conversation

@panavenue
Copy link
Contributor

  1. Setup Metric meter and records
  2. Add 3 new metrics for initial setup
  • connect_latencies
  • closed_connection_count
  • open_connections

@panavenue panavenue requested a review from a team as a code owner November 4, 2025 23:18
@panavenue panavenue force-pushed the system_metrics_setup branch from 8747506 to 1f835d8 Compare November 4, 2025 23:46
@panavenue panavenue force-pushed the system_metrics_setup branch from 1f835d8 to ad4a6e6 Compare November 4, 2025 23:55
@panavenue panavenue requested a review from enocom November 14, 2025 19:02
Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see the AlloyDB implementation was helpful. Left a couple of comments.

disableBuiltInMetrics bool

// clientOpts are options for all Google Cloud API clients.
clientOpts []option.ClientOption
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this anymore, and can just create a client directly with some improvements in the underlying exporter.

See GoogleCloudPlatform/alloydb-go-connector#728.

ApplicationName: d.applicationName,
Region: inst.Region(),
ClientRegion: "Client-Region-Testing", // TODO: detect client region
ComputePlatform: "Compute-Platform-Testing", // TODO: detect compute platform
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n := c.openConnsCount.Add(1)
trace.RecordOpenConnections(ctx, int64(n), d.dialerID, cn.String())
trace.RecordDialLatency(ctx, icn, d.dialerID, latency)
mr.RecordOpenConnection(ctx, attrs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug in the AlloyDB implementation around open connections, but wow I can't find it yet. Beware!

Copy link
Contributor Author

@panavenue panavenue Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think I might know what the potential bug -> The GAUGE metric always have more "open" than "close", because we are sending the metric ONLY on every 60s, and when the application shut down, we never record the "-1" of the open_conn GAUGE metric.

Is this the potential bug? (I was testing on how to go around this, but it's gonna be tough to mitigate this one)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants